feat: add StreamModules to ConsoleService #2931
Conversation
deniseli
requested review from
a team and
wesbillman
and removed request for
a team
| configs = append(configs, &pbconsole.Config{ | ||
| Config: c, | ||
| }) | ||
| configs = append(configs, configFromDecl(decl)) |
There was a problem hiding this comment.
This pattern is much nicer here :)
| config := decl.ToProto().(*schemapb.Config) | ||
| return &pbconsole.Config{ | ||
| Config: config, | ||
| } |
There was a problem hiding this comment.
uber nit/question, can we just use decl.ToProto().(*schemapb.Config) directly here and in the similar places below?
return &pbconsole.Config{
Config: decl.ToProto().(*schemapb.Config),
}There was a problem hiding this comment.
great idea - done! and the linter didn't even complain :)
| return module, nil | ||
| } | ||
|
|
||
| func moduleFromDecls(decls []schema.Decl, sch *schema.Schema) (*pbconsole.Module, error) { |
There was a problem hiding this comment.
Should we add some tests (can be in another PR) for some of this stuff? Or is it covered by other tests atm?
There was a problem hiding this comment.
Done! I only added an integration test for now and didn't test for the contents of the various <blahblah>FromDecl methods since we're going to change so much of that anyway.
| repeated Database databases = 9; | ||
| repeated Enum enums = 10; | ||
| repeated FSM fsms = 11; | ||
| repeated Topic topics = 12; | ||
| repeated TypeAlias typealiases = 13; | ||
| repeated Subscription subscriptions = 14; |
| import type { Module } from '../../protos/xyz/block/ftl/v1/console/console_pb' | ||
|
|
||
| const streamModulesKey = 'streamModules' | ||
| const updateIntervalMs = 60 * 1000 |
There was a problem hiding this comment.
Should we pass this in and default it? Also, is this the correct number of nano seconds (I might have had this wrong before too 😂)?
Math, mostly for me...
1 nano = 1,000,000 milliseconds
60 * 1000 * 1000 = 60,000,000
So I think this is updating at 60ms which might be faster than we want.
There was a problem hiding this comment.
Oof thanks for catching that nano issue!
In retrospect, I'm going to just remove this and leave it totally controlled by the server side. It seems like something we'll be able to optimize a lot more cleanly without having to also conform to potentially several distinct client-side refresh thresholds...
| } catch (error) { | ||
| if (error instanceof ConnectError) { | ||
| if (error.code !== Code.Canceled) { | ||
| console.error('Console service - streamEvents - Connect error:', error) |
There was a problem hiding this comment.
streamEvents -> streamModules
Part 1 of #2805
Next:
getendpoint